Skip to content

feat(jwt): public API — IUserJwtInvalidatedListener, updateUserJwt, login JWT (5/6)#2633

Merged
nan-li merged 8 commits intofeat/identity_verification_feature_flagged_major_releasefrom
feat/iv-public-api-05
May 7, 2026
Merged

feat(jwt): public API — IUserJwtInvalidatedListener, updateUserJwt, login JWT (5/6)#2633
nan-li merged 8 commits intofeat/identity_verification_feature_flagged_major_releasefrom
feat/iv-public-api-05

Conversation

@nan-li
Copy link
Copy Markdown
Contributor

@nan-li nan-li commented May 1, 2026

Description

One Line Summary

PR 5 of 6 against the Identity Verification integration branch — exposes the developer-facing IV API (IUserJwtInvalidatedListener, updateUserJwt, login JWT storage) and bridges PR 3's JwtTokenStore invalidation events to a multi-subscriber EventProducer on UserManager.

Details

Motivation

PR 3 moved the JWT-invalidation event source onto JwtTokenStore.invalidateJwt and removed the cross-cutting setJwtInvalidatedHandler from IOperationRepo. The replacement was supposed to be: UserManager subscribes to JwtTokenStore directly, and exposes the developer-facing event surface. PR 4 added executor-side JWT attachment but left the API unwired.

This PR closes the loop:

  1. Stores the JWT. OneSignal.login(externalId, jwt) finally writes the token to JwtTokenStore so PR 4's executor extensions can find it. Also handles same-user-with-new-token (refresh) and adds a dedicated updateUserJwt(externalId, token) method for the post-401 refresh case.
  2. Surfaces invalidation events. JwtTokenStore.onJwtInvalidatedUserManagerEventProducer<IUserJwtInvalidatedListener> → developer listener. Multi-subscriber (matches the codebase's existing EventProducer idiom). Async dispatch on Dispatchers.Default so the developer's callback doesn't run on the SDK's internal thread.
  3. Listener replay for late registration (fix(jwt): race conditions and IAM reliability issues with identity verification enabled #2613 commit 2 equivalent). If the SDK invalidates a JWT before the developer's listener is wired up (typical during early app startup), the cached event is replayed to listeners that subscribe later. Cleared on logout.

Scope

New public types (in com.onesignal):

  • IUserJwtInvalidatedListenerfun interface for Kotlin SAM-conversion convenience
  • UserJwtInvalidatedEvent(externalId: String) — event payload

IOneSignal gains three methods:

  • updateUserJwt(externalId, token) — store JWT + force-execute the queue so deferred ops dispatch with the fresh token.
  • addUserJwtInvalidatedListener(listener) — multi-subscriber, with replay on late registration.
  • removeUserJwtInvalidatedListener(listener).

The pre-existing login(externalId, jwtBearerToken) overload + loginSuspend were already in IOneSignal; this PR finally wires the JWT param through to JwtTokenStore.

UserManager:

  • Implements IJwtUpdateListener and subscribes to JwtTokenStore in init — replaces PR 3's setJwtInvalidatedHandler bridge with a direct event-source subscription.
  • Holds an EventProducer<IUserJwtInvalidatedListener> and a CoroutineScope(SupervisorJob() + Dispatchers.Default) for async fan-out.
  • fireJwtInvalidated(externalId) — caches the event for replay, schedules async fire with per-listener runCatching (one throwing subscriber doesn't poison others — matches the same pattern from PR 3's JwtTokenStore.invalidateJwt fix).
  • addJwtInvalidatedListener — subscribes, then if lastJwtInvalidatedExternalId != null, schedules a replay to the new listener.
  • clearLastJwtInvalidated() — clears the replay cache. Called from OneSignalImp.logout() / logoutSuspend() so a stale event from a previous user doesn't fire for a new session.
  • New constructor param: JwtTokenStore. DI registration adds provides<UserManager>() so OneSignalImp can resolve the concrete class for listener delegation.

LoginHelper:

  • New constructor param: JwtTokenStore.
  • In switchUser(externalId, jwtBearerToken):
    • New-user path: stores the JWT before userSwitcher.createAndSwitchToNewUser so when the queued LoginUserOperation dispatches, the JWT lookup in hasValidJwtIfRequired already succeeds.
    • Same-user path (no switch needed): still updates the stored token if a new JWT was supplied — supports developers calling login(sameId, refreshedJwt) to refresh.
  • Class visibility tightened to internal class (was public) since the constructor now exposes internal JwtTokenStore.

OneSignalImp:

  • Injects JwtTokenStore via services.getService<JwtTokenStore>() (it was already in DI from PR 1).
  • Implements the three new IOneSignal methods, delegating listener add/remove to services.getService<UserManager>().
  • logout() and logoutSuspend() call clearLastJwtInvalidated() before the user-switch so subsequent listener registrations don't replay the previous user's invalidation.

Layering / consistency

  • Replaces PR 3's plumbing setter (setJwtInvalidatedHandler is already gone from IOperationRepo); the bridge is now JwtTokenStore.IJwtUpdateListener.onJwtInvalidated → UserManager → EventProducer.
  • Migration scenario from feat: identity verification 5.8 #2599 (HYDRATE-IV-required + existing externalId + no JWT → fire listener) is intentionally not ported — verified iOS does not do this either (only fires on actual 401s). Customers enabling IV are expected to ship code that supplies JWTs for active users; the SDK only signals on actual 401 responses.

What is NOT in this PR

  • Logout IV-aware behavior (disable push sub on IV-required logout) + IAM IV integration + RYW plumbing — PR 6 (final).
  • The LoginHelper.switchUser JWT optimization for the migration scenario — out of scope (we don't fire the migration event, see above).

Testing

Unit testing

UserManagerTests (4 new):

  • JwtTokenStore.invalidateJwt fires registered IUserJwtInvalidatedListener — end-to-end via the real JwtTokenStore + UserManager subscription.
  • listener replay: subscribers added after fire receive the most recent event — verifies the replay cache is consulted on addJwtInvalidatedListener.
  • clearLastJwtInvalidated stops replay for new subscribers — verifies the logout-path clears the cache.
  • removeJwtInvalidatedListener stops further notifications — verifies unsubscribe.

LoginHelperTests (2 new):

  • login with JWT stores token in JwtTokenStore before enqueueing op — new-user path + JWT storage.
  • login with same externalId + new JWT updates the stored token — same-user-refresh path; asserts no user-switch happens but JWT is updated.

All affected tests pass locally. Two pre-existing SDKInitTests failures are unrelated and consistent with the integration branch baseline.

Manual testing

Not applicable for this PR — the wired listener fires only when JwtTokenStore.invalidateJwt is called, which happens from PR 4's handleFailUnauthorized in response to a 401. End-to-end manual testing requires the integration branch to be merged + a backend with jwt_required=true.

Affected code checklist

  • Notifications
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes (new: IUserJwtInvalidatedListener, UserJwtInvalidatedEvent, IOneSignal.updateUserJwt, IOneSignal.addUserJwtInvalidatedListener, IOneSignal.removeUserJwtInvalidatedListener. Existing login(externalId, jwt) semantics: JWT now actually stored.)

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing — wires the developer-facing IV API on top of PR 3's invalidation event source and PR 4's executor JWT attachment.
  • Any Public API changes are explained in the PR details and conform to existing APIs (matches the addObserver/removeObserver shape used by IUserStateObserver, etc.)

Testing

  • I have included test coverage for these changes
  • All automated tests pass locally (pre-existing SDKInitTests failures are unrelated — same 2 on integration branch)
  • Manual testing N/A — wiring is consumed only when 401s drive JwtTokenStore.invalidateJwt, which requires a live IV-enabled backend.

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

@nan-li nan-li force-pushed the feat/iv-public-api-05 branch from eb9c4e2 to 0fde95d Compare May 1, 2026 07:52
@nan-li
Copy link
Copy Markdown
Contributor Author

nan-li commented May 1, 2026

@claude review

@nan-li nan-li force-pushed the feat/iv-http-executors-04 branch 2 times, most recently from 50aba1d to 26abaf3 Compare May 4, 2026 04:54
@nan-li nan-li force-pushed the feat/iv-public-api-05 branch from 21e9999 to dcf7f9b Compare May 4, 2026 04:56
@nan-li nan-li force-pushed the feat/iv-http-executors-04 branch from 26abaf3 to e76fb60 Compare May 4, 2026 16:15
@nan-li nan-li force-pushed the feat/iv-public-api-05 branch from dcf7f9b to 0fbd10c Compare May 4, 2026 17:12
@nan-li nan-li force-pushed the feat/iv-http-executors-04 branch from e76fb60 to d2033ea Compare May 4, 2026 17:36
@nan-li nan-li force-pushed the feat/iv-public-api-05 branch 2 times, most recently from 7916683 to a038970 Compare May 4, 2026 19:24
Copy link
Copy Markdown
Contributor

@abdulraqeeb33 abdulraqeeb33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one nit but otherwise looking great

* that synchronously trigger invalidation don't run app code on the SDK's internal thread.
* Replay (synchronous, on the calling thread) bypasses this scope.
*/
private val jwtInvalidatedDispatchScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use existing Default scope from OneSignalDispatcher

@nan-li nan-li force-pushed the feat/iv-public-api-05 branch from a038970 to c233c25 Compare May 5, 2026 15:57
@nan-li nan-li force-pushed the feat/iv-http-executors-04 branch from d2e64df to c8a6b9a Compare May 6, 2026 20:24
@nan-li nan-li force-pushed the feat/iv-public-api-05 branch from c233c25 to eb44594 Compare May 6, 2026 20:25
@nan-li nan-li force-pushed the feat/iv-public-api-05 branch from c3a683f to 8c4c64a Compare May 7, 2026 01:21
@nan-li nan-li force-pushed the feat/iv-http-executors-04 branch from c8a6b9a to 6d18028 Compare May 7, 2026 20:47
Base automatically changed from feat/iv-http-executors-04 to feat/identity_verification_feature_flagged_major_release May 7, 2026 20:47
nan-li and others added 4 commits May 7, 2026 13:48
…gin JWT (5/6)

Adds the developer-facing IV API:
- IUserJwtInvalidatedListener (fun interface) + UserJwtInvalidatedEvent
- IOneSignal.updateUserJwt(externalId, token)
- IOneSignal.addUserJwtInvalidatedListener / removeUserJwtInvalidatedListener
- LoginHelper stores the JWT in JwtTokenStore on switchUser (new user + same-user-new-token)

UserManager subscribes to JwtTokenStore as IJwtUpdateListener; onJwtInvalidated
forwards to a multi-subscriber EventProducer<IUserJwtInvalidatedListener>
fired async on Dispatchers.Default with per-listener runCatching.

Listener replay: caches the last invalidation event so listeners that subscribe
after the invalidation still receive the signal. Cleared on logout.
Rewrite the listener replay to match the reference design from #2613
commit 89cca43:
- Lock-protected pendingJwtInvalidatedExternalId (consume-on-first-subscribe)
  replaces the @volatile lastJwtInvalidatedExternalId (always-replay)
- fireJwtInvalidated buffers ONLY when no subscribers exist; otherwise
  schedules an async fire. Closes the double-fire race.
- onModelReplaced clears the buffer on IdentityModel replacement (login or
  logout switch via UserSwitcher.createAndSwitchToNewUser → replace).
- Drop the explicit clearLastJwtInvalidated() method and its calls in
  OneSignalImp.logout() / logoutSuspend() — clear is now automatic.

Match #2599 conventions:
- Drop the isInitialized throw in addUserJwtInvalidatedListener /
  removeUserJwtInvalidatedListener.
- Add updateUserJwtSuspend.

Update IUserJwtInvalidatedListener docstring to clarify replay is
synchronous on the caller's thread; regular fire is async.

Tests rewritten for the new semantics: first-subscriber-replay,
consume-on-first, fire-with-subscribers-doesn't-buffer, onModelReplaced
clears.
Match the init-readiness pattern used by updateUserJwt: when background
threading is enabled, waitForInit; otherwise throw if not initialized.
Without this, listeners added before initWithContext silently bind to a
not-yet-wired UserManager.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same-user-id login with a fresh JWT (the documented post-401 refresh
path) was storing the new token but never calling forceExecuteOperations,
so ops parked by hasValidJwtIfRequired stayed deferred until something
else woke the queue. updateUserJwt already does this correctly; reference
#2599 LoginHelper does too. Match that design — drop redundant
if (jwtBearerToken != null) guards (putJwt no-ops on null) and call
forceExecuteOperations unconditionally on the same-id branch so the
queue drains as soon as the developer supplies a fresh token.

Extend the existing same-id+JWT test to verify forceExecuteOperations
fires.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nan-li and others added 4 commits May 7, 2026 13:48
Three review-driven fixes on the IV public-API surface:

1. OneSignal.kt: add @JvmStatic wrappers for updateUserJwt,
   addUserJwtInvalidatedListener, removeUserJwtInvalidatedListener, and
   updateUserJwtSuspend. Without these, the four new IOneSignal methods
   were unreachable from app code (the OneSignal object is the documented
   Java/Kotlin entry point and "implements IOneSignal in spirit"). Mirrors
   the convention from reference branch #2599.

2. OneSignalImp.kt: mask jwtBearerToken in login()/loginSuspend() DEBUG
   logs (...${jwtBearerToken?.takeLast(8)}). Pre-PR the parameter was a
   no-op (LoginHelper had a TODO) so the leak was theoretical; this PR
   wires the token through to JwtTokenStore.putJwt, so a live bearer
   credential now flows through Logging.log at DEBUG. updateUserJwt
   already masks; mirror that.

3. UserManager.kt: restore Logging.warn(msg, ex) form on the two new
   runCatching.onFailure handlers in fireJwtInvalidated and
   addJwtInvalidatedListener. Interpolating ${ex.message} drops the
   stack trace; Logging.warn already accepts a Throwable second arg
   that propagates through to log listeners and Otel — same pattern
   restored in JwtTokenStore via e76fb60.

Also refreshes detekt baseline for the new MagicNumber (takeLast(8)),
ConstructorParameterNaming (_jwtTokenStore on UserManager), TooManyFunctions
(UserManager now also implements IJwtUpdateListener), and UseCheckOrError
on the 4 new IllegalStateException throws.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s iOS

The buffer-and-consume design (pendingJwtInvalidatedExternalId +
jwtInvalidatedLock + consume-on-first-subscribe replay) was added to
handle cold-start 401s when no listener was subscribed yet. iOS doesn't
do this — it fires only to currently-subscribed listeners, late
subscribers miss earlier events. Match iOS: drop the buffer entirely.

Reverts the structure introduced by d137481 ("align jwt-invalidated
replay with #2613 buffer-and-consume"). The simpler design here matches
reference branch #2599.

Side effects:
- onModelReplaced becomes a no-op (no buffer to clear). Kept the
  override since ISingletonModelStoreChangeHandler requires it.
- fireJwtInvalidated now uses OneSignalDispatchers.launchOnDefault per
  maintainer request (#3184062053) instead of a custom
  CoroutineScope(SupervisorJob() + Dispatchers.Default).
- KDoc on IOneSignal.addUserJwtInvalidatedListener and the OneSignal.kt
  facade now spell out the pure-pub/sub semantics: "Subscribe early
  (e.g. in Application.onCreate) to avoid missing cold-start 401s."

Test changes: drop the 4 buffer/replay-specific tests
(listener-replay-buffered-event, consume-on-first-subscribe,
fire-with-subscribers-no-buffer, onModelReplaced-clears-buffer).
Replace with one test that confirms late subscribers don't receive
earlier events.
…Store

JwtTokenStore is the source of truth for invalidation events (its
invalidateJwt method is what fires both internal and developer-facing
notifications). It already implements IEventNotifier<IJwtUpdateListener>
and dispatches to internal listeners. Adding a second EventProducer for
the developer-facing IUserJwtInvalidatedListener is consistent with the
class's existing notifier shape — just a different audience.

This eliminates the bridge that lived in UserManager:
- UserManager loses IJwtUpdateListener, _jwtTokenStore ctor param,
  jwtInvalidatedNotifier, addJwtInvalidatedListener,
  removeJwtInvalidatedListener, fireJwtInvalidated, onJwtInvalidated.
- OneSignalImp.{add,remove}UserJwtInvalidatedListener now route directly
  to JwtTokenStore.

Side benefits:
- No eager-construction problem: JwtTokenStore is already pulled in by
  OperationRepo (an IStartableService), so the bridge is live before
  any 401 can dispatch — no IBootstrapService registration needed for
  any class.
- UserManager goes back to being purely about user state.
- Improves on reference branches #2599 / #2613 which kept the bridge in
  UserManager and accepted an eager-construction-via-IAM dependency.

Tests:
- Move the 3 JWT-listener tests from UserManagerTests to
  JwtTokenStoreTests (invalidation fires listener, late subscriber
  doesn't get replay, removeListener stops notifications).
- Drop _jwtTokenStore param from UserManager construction sites.
@nan-li nan-li force-pushed the feat/iv-public-api-05 branch from 8c4c64a to a463522 Compare May 7, 2026 20:50
@nan-li nan-li merged commit bea86f6 into feat/identity_verification_feature_flagged_major_release May 7, 2026
2 of 3 checks passed
@nan-li nan-li deleted the feat/iv-public-api-05 branch May 7, 2026 20:50
@nan-li nan-li mentioned this pull request May 7, 2026
11 tasks
<ID>ConstructorParameterNaming:UserBackendService.kt$UserBackendService$private val _httpClient: IHttpClient</ID>
<ID>ConstructorParameterNaming:UserManager.kt$UserManager$private val _customEventController: ICustomEventController</ID>
<ID>ConstructorParameterNaming:UserManager.kt$UserManager$private val _identityModelStore: IdentityModelStore</ID>
<ID>ConstructorParameterNaming:UserManager.kt$UserManager$private val _jwtTokenStore: JwtTokenStore</ID>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Two detekt baseline entries added/amended in this PR are now stale: line 158 suppresses ConstructorParameterNaming for _jwtTokenStore: JwtTokenStore, but UserManager.kt:26-32 has no such constructor parameter, and line 416 keys TooManyFunctions on UserManager : IUserManagerISingletonModelStoreChangeHandlerIJwtUpdateListener, but UserManager.kt:32 declares only : IUserManager, ISingletonModelStoreChangeHandler<IdentityModel>IJwtUpdateListener is not implemented. Both entries suppress non-existent warnings (created by an earlier version of this PR that was reverted by refactor commits 46d525f / 2a7edec which moved listener machinery into JwtTokenStore) and will silently mask any future legitimate detekt findings against UserManager. Fix: remove both lines, or regenerate the baseline.

Extended reasoning...

What the bug is

Two entries in OneSignalSDK/detekt/detekt-baseline-core.xml were added or amended by this PR but reference UserManager state that does not exist in the current code:

  1. Line 158 (added by this PR per the diff hunk + <ID>ConstructorParameterNaming:UserManager.kt$UserManager$private val _jwtTokenStore: JwtTokenStore</ID>) suppresses a ConstructorParameterNaming finding for a _jwtTokenStore: JwtTokenStore constructor parameter on UserManager.
  2. Line 416 (amended by this PR — diff shows -IUserManagerISingletonModelStoreChangeHandler+IUserManagerISingletonModelStoreChangeHandlerIJwtUpdateListener) suppresses a TooManyFunctions finding keyed on UserManager implementing IJwtUpdateListener.

Step-by-step proof

  1. Read UserManager.kt:26-32. The constructor signature is:
    internal open class UserManager(
        private val _subscriptionManager: ISubscriptionManager,
        private val _identityModelStore: IdentityModelStore,
        private val _propertiesModelStore: PropertiesModelStore,
        private val _customEventController: ICustomEventController,
        private val _languageContext: ILanguageContext,
    ) : IUserManager, ISingletonModelStoreChangeHandler<IdentityModel>
    There is no _jwtTokenStore: JwtTokenStore parameter, and the implemented-interface list is exactly IUserManager, ISingletonModelStoreChangeHandler<IdentityModel>IJwtUpdateListener is absent.
  2. grep for JwtTokenStore in UserManager.kt returns no matches.
  3. Detekt's baseline keys for ConstructorParameterNaming and TooManyFunctions include the constructor parameter list and the implemented-interface signature respectively. The keys at lines 158 and 416 do not match the class as it now exists, so the suppressions never apply.

Why this happened

This PR family went through several refactors. Earlier iterations injected JwtTokenStore into UserManager and made UserManager implement IJwtUpdateListener to bridge invalidation events to a UserManager-owned EventProducer<IUserJwtInvalidatedListener>. Subsequent commits — notably 46d525f ("refactor(iv): drop jwt-invalidated buffer/replay; pure pub/sub matches iOS") and 2a7edec ("refactor(iv): move IUserJwtInvalidatedListener notifier into JwtTokenStore") — moved the listener machinery directly into JwtTokenStore and removed both the constructor parameter and the interface implementation. The detekt baseline was not regenerated, so the suppressions added for the earlier shape remain.

Impact

No runtime impact — these are pure baseline-suppression entries. The cosmetic harm is real, though: stale entries silently mask any future legitimate detekt findings against UserManager's constructor naming or implemented-interface count. A future engineer adding a JWT-related parameter would see the warning suppressed without realizing why; a future refactor that legitimately bumps UserManager's function count past the threshold would have its TooManyFunctions warning auto-suppressed by a key that only happens to match an unrelated past shape. Detekt baselines exist to lock in known findings; stale entries undermine that contract.

Fix

Mechanical: delete the two stale lines from detekt-baseline-core.xml (line 158 and the IJwtUpdateListener portion of line 416, restoring it to UserManager : IUserManagerISingletonModelStoreChangeHandler if that finding still applies, or remove the line entirely if it does not). Alternatively, regenerate the baseline with ./gradlew detektBaseline and let detekt emit only the entries that match the current code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants